Skip to content

Move BOLT11 JIT params to payment metadata#899

Open
tnull wants to merge 4 commits intolightningdevkit:mainfrom
tnull:2026-05-move-jit-params-to-payment-metadata
Open

Move BOLT11 JIT params to payment metadata#899
tnull wants to merge 4 commits intolightningdevkit:mainfrom
tnull:2026-05-move-jit-params-to-payment-metadata

Conversation

@tnull
Copy link
Copy Markdown
Collaborator

@tnull tnull commented May 6, 2026

Context

We recently found that for the intended BOLT12-JIT flow we'll have to encode the LSPS2 parameters in the payment metadata (based on lightningdevkit/rust-lightning#4584). As a prefactor to that (and to simplify things for #811, rather than add yet another store type there, just to revert once we get to the BOLT12-JIT changes), we here switch to store the LSPS2 parameters in the BOLT11 payment_metadata rather than persisting them on-disk. To make this safe we'll also need lightningdevkit/rust-lightning#4528, as otherwise the payer could collude with the LSP to rob the payee.

Summary

This moves LSPS2/JIT-channel receive parameters out of dedicated payment-store
state and into the BOLT11 invoice payment_metadata, so the payment store no
longer needs a PaymentKind::Bolt11Jit variant for new payments.

Changes

  • Rename LSPFeeLimits to LSPS2Parameters.
  • Add Bolt11PaymentMetadata in bolt11.rs with TLV-based encoding.
  • Encode LSPS2Parameters into BOLT11 payment_metadata when creating JIT
    invoices.
  • Store new JIT receives as PaymentKind::Bolt11.
  • Decode legacy PaymentKind::Bolt11Jit records as PaymentKind::Bolt11.
  • Reject skimmed inbound payments unless valid BOLT11 payment_metadata proves
    the LSPS2 fee is within the negotiated limit.
  • Drop the stale top-level PaymentDetails field-1 JIT metadata reader, while
    keeping the released legacy PaymentKind::Bolt11Jit decoder.
  • Add a changelog note that pending JIT payments may fail after upgrade because
    prior JIT state is not migrated.

The values describe the `LSPS2` parameters negotiated for JIT-channel
receives, not a fee-limit concept owned by the payment store.

Rename the public type and internal references while preserving the
existing fields, TLV encoding, and `PaymentKind::Bolt11Jit` storage
shape for the follow-up refactor.

Co-Authored-By: HAL 9000
@tnull tnull requested a review from joostjager May 6, 2026 12:09
@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented May 6, 2026

👋 Thanks for assigning @joostjager as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

Copy link
Copy Markdown
Contributor

@joostjager joostjager left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR looks great. I like the offloading of state to clients. And ofc happy to see usage of lightning/bolts#912 😄

Comment thread src/payment/bolt11.rs Outdated
let payment_hash = invoice.payment_hash();
let payment_secret = invoice.payment_secret();
let lsp_fee_limits = LSPFeeLimits {
let lsp_fee_limits = LSPS2Parameters {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this rename is strictly better. Parameters sounds broader than what it is.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's intentional as we might add more fields in the BOLT12 context that are not 'fee limits'. Sorry, maybe should have given that rationale in the commit description.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which parameters are that?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/liquidity.rs
let client_payment = client_node.payment(&client_payment_id).unwrap();
match client_payment.kind {
PaymentKind::Bolt11Jit { counterparty_skimmed_fee_msat, .. } => {
PaymentKind::Bolt11 { counterparty_skimmed_fee_msat, .. } => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be worth to add some coverage for "skimmed fee with missing/malformed metadata" failing?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a small fixup that basically asserts that, let me know if you deem this sufficient. Note that asserting handle_event behavior overall is a bit tricky, and we don't have a good way to create integration tests that feature LSPs maliciously withholding funds.

@tnull tnull force-pushed the 2026-05-move-jit-params-to-payment-metadata branch from 7d41d4f to 40f11e6 Compare May 8, 2026 11:29
@tnull
Copy link
Copy Markdown
Collaborator Author

tnull commented May 8, 2026

Now updated and included a commit that allows us to de/encrypt PaymentMetadata, as we should do that given that otherwise the payer will see whatever we put into that. Currently the LSP parameters aren't super sensitive, but especially given we're adding the general struct for expandibility, we want to make sure encrypt from the start to avoid any privacy leakage.

@tnull tnull requested a review from joostjager May 8, 2026 11:31
Comment thread src/payment/metadata.rs
}
}

fn nonce(&self, payment_hash: &PaymentHash, payment_secret: &PaymentSecret) -> [u8; 12] {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it totally ruled out that payment hash and secret are never reused, also not in some manual flow for example? Perhaps defensively picking a random nonce and storing it in the metadata is safer?

Comment thread src/payment/metadata.rs
impl PaymentMetadataKeys {
pub(crate) fn new(base_secret: [u8; 32]) -> Self {
Self {
encryption_key: hmac_sha256(&base_secret, b"ldk_node_payment_metadata_encryption_key"),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With random nonces, this derivation isn't necessary anymore because base_secret is just the encryption key, I think

Comment thread src/payment/metadata.rs
let nonce = keys.nonce(payment_hash, payment_secret);
let mut ciphertext = sealed::PaymentMetadataTlv::from(self.clone()).encode();
let cipher = ChaCha20Poly1305::new(Key::new(keys.encryption_key), Nonce::new(nonce));
let tag = cipher.encrypt(&mut ciphertext, Some(PAYMENT_METADATA_AAD));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really check to see if anything useful can be exposed from lightning/src/crypto rather than doing it again here. And maybe also just drop encryption from this PR until there is an answer to that.

Comment thread src/liquidity.rs
Comment thread src/liquidity.rs
}]);

let payment_metadata = PaymentMetadata { lsps2_parameters: Some(lsps2_parameters) }
.encrypt(&payment_metadata_keys, &payment_hash, &payment_secret)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For bolt12, I think the encryption happens on the blinded path level? Is there anything in the design of this PR that would then double-encrypt when bolt12 is added?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants